Class-based approach for typed object model #11
Conversation
|
Great find, this is really interesting! I'd love to get rid of interface MyClass extends Ember.Object { ... }
const MyClass: EmberClass<MyClass>;And will now use this instead: // if no interfaces/mixins are needed:
class MyClass extends Ember.Object { ... }
// if interfaces/mixins are needed:
interface MyClass extends SomeInterfaceOrMixin { ... }
class MyClass extends Ember.Object { ... }That looks much better! |
| @@ -9,7 +9,7 @@ proxy.get('firstObject'); // 'amoeba' | |||
|
|
|||
| const overridden = Ember.ArrayProxy.create({ | |||
| content: Ember.A(pets), | |||
| objectAtContent(idx) { | |||
| objectAtContent(idx: number) { | |||
There was a problem hiding this comment.
One side effect I can see is that arguments to method overrides are no longer inferred. For instance since we know the argument types to setupController we shouldn't have to specify them:
// working in EmberClass<T> approach, not working in ES6 class approach
const MyRoute1 = Ember.Route.extend({
setupController(controller, model) {
controller.set('model', model);
}
});
// not working in either approach
class MyRoute2 extends Ember.Route {
setupController(controller, model) {
controller.set('model', model);
}
}After reading microsoft/TypeScript#6118 and the linked issues, typescript surprisingly doesn't support this! So rather than implementing it in our bespoke object model, I'd rather remove it and wait for an official solution
There was a problem hiding this comment.
Ah, I totally missed that that was happening before! (And didn't stop to reflect on why I'd unthinkingly made that change in this file). While it's pretty cool that that was working, I'd agree that sticking as close to the vanilla object model as possible will hopefully yield us the best results in the long run :)
Thinking a bit more, rather than relying on merging we could use the same pattern in the definitions as an actual implementation would: class MyClass extends Ember.Object.extend(SomeMixin) { ... }I think we'd need to reverse the order of the clauses in My guess is that this form factor would be more immediately apparent to a reader as to what's happening, and it's a plus to have the declaration code mirror what an actual implementation would look like. |
Yeah, that's a bug in both our branches |
Also support mixin-only `extend()` properly.
Class-based approach for typed object model
I started to take a pass at introducing generics for enumerables like
Ember.ArrayProxybased on the changes in #1 , but found that the approach there prohibited usage like:The reason for that is that there's no notion of a polymorphic constant in TypeScript, i.e. there's no way to do something like
const ArrayProxy<T>: EmberClass<ArrayProxy<T>>.The main notable change is the removal of the
EmberClass<T>interface in favor of declaring actual ES6 classes. I'm also taking advantage of the class/interface merging trick mentioned in microsoft/TypeScript#340 (comment) to avoid needing to repeat interface methods in each class definition.@dwickern if you have some time to play with this at some point, I believe I've captured the same safety guarantees as you have with the
EmberClass<T>approach, but I haven't worked with TS's type system extensively, so it's entirely possible I've missed something :)